Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

examples/[pis]cscf: adds example database structures for the P/I/S-CSCFs #718

Closed
wants to merge 1 commit into from

Conversation

vingarzan
Copy link
Contributor

This helps first timers and casual users a lot, as they won't have to dig
through the history of Kamailio and various modules to find these
structures.

This helps first timers and casual users a lot, as they won't have to dig
through the history of Kamailio and various modules to find these
structures.
@ngvoice
Copy link
Member

ngvoice commented Jul 20, 2016

Hi Dragos,

not sure about this one:
The "proper" way would be, to add them to the regular "kamdbctl" scripts, where they are currently missing. It's likely a policy question: Should we add SQL build-scripts to the examples? Then that would apply to other examples as well.

I suggest @miconda should Merge or comment on this.

Thanks,
Carsten

@miconda
Copy link
Member

miconda commented Jul 21, 2016

Perhaps we have to decide how to deal with the examples that need extra db tables.

I am fine having an sql file dedicated for them. Perhaps such examples can be in their own subfoder, have the cfg, sql and a short readme. Many configs related to the same topic can be in same folder, so I think the ims stuff is more or less like this.

I will not go to add them to the kamdbctl, because some of example using sqlops may be dependent on database server with some of its custom queries (e.g., if using REPLACE in cfg, then mysql is required).

If anyone has other options, let's discuss.

Otherwise I am fine with merging this one -- as said, ims examples have their own subfolder.

@vingarzan
Copy link
Contributor Author

I am fine with either or even none.

It was though quite nasty to find a few days ago the create statements for something standard and simple like the pua module.The README said "The SQL syntax to create it can be found in presence_xml-create.sql script in the database directories in the kamailio/scripts folder." Yeah, right :). I ended up digging through very old history to find where that file went missing. Then I manually patched and extended it to match what the code was using. And now I still don't have the indices...

In Wharf/OpenEPC we have yet another policy, which seems to work though - every module that uses a DB must have a directory "sql" inside the module that contains the said structures. I know that this combines code with SQL, bash scripts and other things, but IMO it is better to have all related things localized than spread around and lost eventually.

So while the examples dir is nice, the issues with it is that it gets too disconnected from the actual code. If things are closer-by, then devs have no excuse that they forgot to patch-in a new column in the DB, or a new parameter in the example config. Anyway, just my 2 cents, I don't want to make big waves for this little thing and waste your time.

@ngvoice
Copy link
Member

ngvoice commented Sep 19, 2016

I just added a note about the database structure (as well as the I-CSCF schema), i guess we can close this topic.

@miconda
Copy link
Member

miconda commented Sep 19, 2016

@ngvoice - ok thanks!

@miconda miconda closed this Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants